Skip to content

fix(platform-wallet): require only atomic tracked-lock writes for reconciliation - #5207

Open
lklimek wants to merge 9 commits into
fix/pr-5126from
fix/sqlite-asset-lock-reconciliation
Open

lklimek wants to merge 9 commits into
fix/pr-5126from
fix/sqlite-asset-lock-reconciliation

Conversation

@lklimek

@lklimek lklimek commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR

When a saved funding deposit is reported as already used, retain its recovery state and return the specific result instead of failing on an unrelated storage requirement.

User story

As a wallet user, I can understand why a saved deposit cannot fund another payment and check the earlier payment or choose another deposit.

Scenario

Choose Fund for a restored deposit that the network reports as already used. The recovery step updates the loaded deposit and saves that update atomically. It must not fail merely because the storage backend cannot restore unrelated wallet data.

Detailed discussion

Issue being fixed or feature implemented

Stacked on #5150 (fix/pr-5126). Integrated in dashpay/dash-evo-tool#1036. SQLite restores tracked asset locks but does not declare the broader WALLET_RESTORE capability. Asset-lock reconciliation previously required that capability despite operating on an already-loaded record.

What was done?

Compatibility

No callback layout, capability version, or Swift/iOS integration changes. Backends need atomic writes and tracked asset-lock persistence for this operation.

How Has This Been Tested?

  • 20 targeted Rust tests passed: 4 capability, 1 FFI callback admission, 3 SQLite reopen/manager hydration, and 12 typed-error/reconciliation tests.
  • Scoped formatting, Clippy with CI's --no-deps -D warnings, and diff checks passed.
  • No live user-wallet spending. Final net diff has no Swift changes.

Breaking Changes

None.

Checklist

  • Self-review performed
  • Relevant operation contract documented
  • Focused regression coverage
  • Validation limitations documented

🤖 Co-authored by Claudius the Magnificent AI Agent

Accept atomic tracked-asset-lock writes with either full wallet restore
or the narrower tracked-asset-lock restore capability. SQLite attests
only the latter, matching its public load and manager hydration paths.
Preserve the typed consumption report, nonterminal recovery marker,
proof, and persistence failure behavior.

Co-Authored-By: Codex GPT-6 <noreply@openai.com>

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: dashpay/platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 560cbd41-2ba2-4329-a2f3-8dcf6b9f956e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw

thepastaclaw commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit 055727f) · triage: low

Make the existing tracked-lock capability cover persistence and nonterminal
restart restore, and require only that contract plus atomic changesets for
reconciliation. Admit the FFI bit only with persistence and paired restore
callbacks. Fail Swift wallet restore if its tracked-lock fetch fails.

Co-Authored-By: Codex <noreply@openai.com>

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
@lklimek lklimek changed the title fix(wallet): allow scoped SQLite asset-lock reconciliation fix(wallet): require tracked asset locks to persist and restore Sep 29, 2026
@lklimek lklimek changed the title fix(wallet): require tracked asset locks to persist and restore fix(platform-wallet): allow scoped SQLite asset-lock reconciliation Sep 29, 2026
@lklimek lklimek changed the title fix(platform-wallet): allow scoped SQLite asset-lock reconciliation fix(wallet)!: require tracked asset locks to persist and restore Sep 29, 2026
Require only atomic tracked-lock persistence for reconciliation of an
already-loaded wallet. Preserve existing capability meanings and FFI host
admission behavior, with wallet restore work outside this change.

Keep regression coverage for SQLite reopen/hydration, typed consumption
errors, and persistence failure handling.

Co-Authored-By: Codex <noreply@openai.com>

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
@lklimek lklimek changed the title fix(wallet)!: require tracked asset locks to persist and restore fix(wallet): require only atomic tracked-lock writes for reconciliation Sep 29, 2026
@lklimek lklimek changed the title fix(wallet): require only atomic tracked-lock writes for reconciliation fix(platform-wallet): require only atomic tracked-lock writes for reconciliation Oct 12, 2026
Take the base's CHANGELOG.md: the base dropped its hand-written Unreleased
section, so the entry for this change comes from the PR title at release.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lklimek
lklimek marked this pull request as ready for review October 12, 2026 09:01
lklimek and others added 2 commits October 12, 2026 09:25
Pick up the storage fixes from the base (cached replay queries, mempool
claimant provenance, replay amount guard, recovery fixtures).

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bring in the base branch at d280e72: rust-dashcore pinned to the
reworked dashpay/rust-dashcore#1112 (844da0e8), the dev-based pin and
Platform adaptations from the dependency PR, and v5.1-dev.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSi8cX229kW2fmLxKW6ir4

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

Verified the complete diff at b30a09e against the supplied PR base and traced capability admission, reconciliation, and SQLite hydration; no in-scope defects were confirmed. The narrower mask matches the atomic tracked-lock writes performed on an already-loaded record without changing capability-bit meanings, FFI layouts, proof handling, or persistence-failure behavior. Validation was static only: the supplied CI snapshot shows successful Kotlin SDK build/tests and policy checks, but does not independently establish the author's reported targeted Rust test results.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained capability-mask relaxation in ASSET_LOCK_RECONCILIATION, with supporting documentation and regression tests rather than intricate changes to funding or recovery logic.
  • Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: glm-5.3-flash — zai quota: 5h 100% left, weekly 85% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left)
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer

@lklimek
lklimek added this pull request to stack #5382 October 12, 2026 12:30
Brings in the sweep-claimant and unsafe-mempool-record repairs from the
base branch (f49527f).

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSi8cX229kW2fmLxKW6ir4

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at head 055727f and found no actionable in-scope defects. The narrower reconciliation mask preserves existing capability semantics and FFI admission behavior, with regression coverage for minimal callback admission, typed consumption errors, and SQLite reopen/manager hydration. Validation was static only; the supplied CI snapshot reports successful Rust workspace, wallet, and Swift checks, with browser shard 1 and Kotlin SDK checks still pending.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained capability-mask correction in ASSET_LOCK_RECONCILIATION with focused regression tests, without changing funding, proof verification, or reconciliation algorithms.
  • Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: glm-5.3-flash — zai quota: 5h 93% left, weekly 99% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants